New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(mail): Fix big logos in mail templates for Outlook #46419

Merged

nickvergessen merged 1 commit into master from bugfix/noid/limit-logo-size-for-outlook

Jul 17, 2024

Member

nickvergessen commented Jul 10, 2024 •

edited

Loading

Case	Before	After
Huge image
Mini image

Todo

Create repair step to cache the image size on update
Adjustments for enterprise logo

Checklist

Code is properly formatted
Sign-off message is added to all commits
Tests (unit, integration, api and/or acceptance) are included
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
Backports requested where applicable (ex: critical bugfixes)

nickvergessen added bug 3. to review feature: theming feature: emails labels

nickvergessen added this to the Nextcloud 30 milestone

nickvergessen requested review from SystemKeeper, susnux and skjnldsv

July 10, 2024 15:58

nickvergessen self-assigned this

susnux approved these changes

View reviewed changes

github-advanced-security bot found potential problems

View reviewed changes

apps/theming/lib/ImageManager.php Fixed Show fixed Hide fixed

SystemKeeper approved these changes

View reviewed changes

nickvergessen force-pushed the bugfix/noid/limit-logo-size-for-outlook branch from ec3f2a7 to 7e6ce49 Compare

July 11, 2024 06:54

Member Author

nickvergessen commented Jul 11, 2024

Added the repair step and pinged the release crew to ask about the enterprise logo

github-advanced-security bot found potential problems

View reviewed changes

apps/theming/lib/ImageManager.php

@@ @@ -186,6 +186,10 @@ @@
               		} catch (NotFoundException $e) {
               		} catch (NotPermittedException $e) {
               		}
+              		if ($key === 'logo') {
+              			$this->config->deleteAppValue('theming', 'logoDimensions');

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::deleteAppValue has been marked as deprecated

nickvergessen force-pushed the bugfix/noid/limit-logo-size-for-outlook branch from 7e6ce49 to 0252003 Compare

July 16, 2024 14:08

github-advanced-security bot found potential problems

View reviewed changes

apps/theming/lib/ImageManager.php

+              		if ($key === 'logo') {
+              			$newImage = @imagecreatefromstring(file_get_contents($tmpFile));
+              			if ($newImage !== false) {
+              				$this->config->setAppValue('theming', 'logoDimensions', imagesx($newImage) . 'x' . imagesy($newImage));

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::setAppValue has been marked as deprecated

apps/theming/lib/ImageManager.php Fixed Show fixed Hide fixed

nickvergessen force-pushed the bugfix/noid/limit-logo-size-for-outlook branch from 0252003 to c9842b6 Compare

July 17, 2024 06:57

github-advanced-security bot found potential problems

View reviewed changes

apps/theming/lib/ImageManager.php

+              			} elseif (str_starts_with($detectedMimeType, 'image/svg')) {
+              				$matched = preg_match('/viewbox=["\']\d* \d* (\d*\.?\d*) (\d*\.?\d*)["\']/i', $content, $matches);
+              				if ($matched) {
+              					$this->config->setAppValue('theming', 'logoDimensions', $matches[1] . 'x' . $matches[2]);

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::setAppValue has been marked as deprecated

apps/theming/lib/ImageManager.php

+              					$this->config->setAppValue('theming', 'logoDimensions', $matches[1] . 'x' . $matches[2]);
+              				} else {
+              					$this->logger->warning('Could not read logo image dimensions to optimize for mail header');
+              					$this->config->deleteAppValue('theming', 'logoDimensions');

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::deleteAppValue has been marked as deprecated

apps/theming/lib/ImageManager.php

+              				}
+              			} else {
+              				$this->logger->warning('Could not read logo image dimensions to optimize for mail header');
+              				$this->config->deleteAppValue('theming', 'logoDimensions');

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::deleteAppValue has been marked as deprecated

nickvergessen force-pushed the bugfix/noid/limit-logo-size-for-outlook branch from c9842b6 to 37154e2 Compare

July 17, 2024 07:18


          fix(mail): Fix big logos in mail templates for Outlook

693a81b

Signed-off-by: Joas Schilling <coding@schilljs.com>

nickvergessen force-pushed the bugfix/noid/limit-logo-size-for-outlook branch from 37154e2 to 693a81b Compare

July 17, 2024 07:25

nickvergessen merged commit 0588899 into master

164 of 166 checks passed

nickvergessen deleted the bugfix/noid/limit-logo-size-for-outlook branch

July 17, 2024 09:54

Member Author

nickvergessen commented Jul 19, 2024

/backport to stable29

backportbot bot added the backport-request label

backportbot bot mentioned this pull request

[stable29] fix(mail): Fix big logos in mail templates for Outlook #46627

Merged

backportbot bot removed the backport-request label

nickvergessen mentioned this pull request

[stable28] fix(mail): Fix big logos in mail templates for Outlook #46628

Merged

blizzz mentioned this pull request

30.0.0 beta 1 #46713

Merged

kesselb mentioned this pull request

[Bug]: custom mail template class is broken in 29.0.5 / 28.0.9 #47430

Closed

8 tasks

joshtrichards mentioned this pull request

E-Mail image sizing wrong in Outlook #32790

Closed

4 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review bug feature: emails feature: theming